Skip to content

refactor(tables): migrate four list tables to useDataTableParams#1269

Merged
anderdc merged 3 commits into
entrius:testfrom
Dexterity104:refactor/migrate-tables-to-useDataTableParams
May 29, 2026
Merged

refactor(tables): migrate four list tables to useDataTableParams#1269
anderdc merged 3 commits into
entrius:testfrom
Dexterity104:refactor/migrate-tables-to-useDataTableParams

Conversation

@Dexterity104
Copy link
Copy Markdown
Contributor

@Dexterity104 Dexterity104 commented May 25, 2026

Summary

Migrate four list tables onto the existing useDataTableParams hook so URL backed sort, page, and filter state share a single helper. The hook already powers TopMinersTable and TopRepositoriesTable, and these four tables were duplicating the same inline boilerplate (manual useSearchParams reads plus useState for sortField/sortDir/page plus hand rolled setPage(0) resets on filter change).

Tables migrated:

  • MinerPRsTable: prPage and prStatus now flow through the hook. Sort state (previously local) is URL backed via sort and dir.
  • MinerOpenDiscoveryIssuesByRepo: same pattern for issuePage and issueStatus.
  • RepositoryPRsTable: prAuthor moved into the hook filters config. Sort and page state move to URL under prSort, prDir, and prPage. The session stored status filter remains a local concern.
  • RepositoryIssuesTable: sort and page state move to URL under issueSort, issueDir, and issuePage. The session stored status filter is unchanged.

All preexisting URL parameter names (prPage, prStatus, issuePage, issueStatus, prAuthor) stay byte identical so shareable links keep working. The DEFAULT_SORT_DIR per field overrides are preserved via the hook's defaultOrderOverrides, and the "reset page on filter change" semantics are preserved via the hook native page reset on setFilter and setSort.

For the two repository tables, the new sort and page params are prefixed (prSort/prDir/prPage and issueSort/issueDir/issuePage) so the Issues and PRs tabs on the repository details page do not stomp on each other when a user switches tabs.

For the miner tables, the existing useEffect keyed on [githubId] still clears local UI state when navigating between miners, and now also clears the URL sort and dir params so each miner opens with the default sort, matching prior behavior.

No new dependencies. No changes to useDataTableParams.

Related Issues

N/A

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Screenshots

No UI or visual changes in this PR, so before/after media is not applicable.

Checklist

  • New components are modularized/separated where sensible
  • Uses predefined theme (e.g. no hardcoded colors)
  • Responsive/mobile checked
  • Tested against the test API
  • npm run format and npm run lint:fix have been run
  • npm run build passes
  • Screenshots included for any UI/visual changes

@xiao-xiao-mao xiao-xiao-mao Bot added the refactor Code restructuring without behavior change label May 25, 2026
@ventura-oss
Copy link
Copy Markdown
Contributor

ventura-oss commented May 29, 2026

Skipping for now — this branch currently conflicts with test (mergeable: CONFLICTING). Could you rebase onto the latest test and resolve the conflicts? Happy to review once it merges cleanly. CI itself is green, so it's just the rebase that's blocking.

anderdc
anderdc requested changes May 29, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@anderdc anderdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repository-table half is the real win — RepositoryPRsTable and RepositoryIssuesTable move sort/page onto useDataTableParams with prefixed params (prSort/prDir/prPage, issueSort/issueDir/issuePage). That's genuine URL persistence and tab isolation. Keep those, and keep the miner tables' status-filter wiring through the hook (it preserves prStatus/issueStatus byte-for-byte).

Drop the URL-backed sort on the two miner tables. In MinerPRsTable.tsx and MinerOpenDiscoveryIssuesByRepo.tsx, sort routes through the hook's bare sort/dir, but the [githubId] effect deletes sort/dir on every mount — so the URL sort never survives a mount and behaves identically to the old local sortField/sortDir, while adding transient params and an extra setSearchParams in the reset effect. Restore sort to local state on both miner tables (with the prior setPage(0) on sort change). That also avoids the miner tables writing to the same unprefixed sort/dir namespace the leaderboard tables use.

Keep:

  • RepositoryPRsTable + RepositoryIssuesTable migration to useDataTableParams.
  • Miner-table status-filter wiring through the hook.

Drop:

  • URL-backed sort (sort/dir) on MinerPRsTable and MinerOpenDiscoveryIssuesByRepo, including the sort/dir deletion added to their [githubId] effects — revert those tables' sort to local state.

@Dexterity104
Copy link
Copy Markdown
Contributor Author

Dexterity104 commented May 29, 2026

Hi @anderdc
Thanks for your review. I reverted sort to local state on both miner tables and dropped the sort/dir cleanup from the [githubId] effect. Status filter still routes through the hook.

@anderdc anderdc merged commit 43f6f57 into entrius:test May 29, 2026
2 checks passed
@Dexterity104 Dexterity104 deleted the refactor/migrate-tables-to-useDataTableParams branch May 29, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anderdc anderdc anderdc approved these changes

Assignees

No one assigned

Labels

refactor Code restructuring without behavior change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Dexterity104 @ventura-oss @anderdc